Skip to content

assert_instr: add support for stricter tests - #2212

Open
RalfJung wants to merge 4 commits into
rust-lang:mainfrom
RalfJung:assert_instr_not
Open

assert_instr: add support for stricter tests#2212
RalfJung wants to merge 4 commits into
rust-lang:mainfrom
RalfJung:assert_instr_not

Conversation

@RalfJung

@RalfJung RalfJung commented Aug 19, 2026

Copy link
Copy Markdown
Member

Motivated by #2208, this extends assert_instr with support for stricter tests:

  • The first commit adds clauses of the form not(instr); the macro will then ensure that the given instruction does not appear in the disassembly.
  • The second commit adds clauses of the form limit(n); the macro will ensure that overall there are no more than n instructions in the disassembly.

I verified that the problematic pack tests indeed fail before the revert of #2033. I am not sure if the not part is even worth it, which is why the 2nd commit removes its uses -- one has to have very specific instructions in mind to exclude them, a general limit on how many instructions total there are seems better? So unless you think I should keep it I plan to remove the first commit from this again.

r? @folkertdev

@RalfJung RalfJung changed the title assert_instr: add support for explicit exclusion of bad instructions assert_instr: add support for stricter tests Aug 19, 2026
@RalfJung

RalfJung commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

I am also confused by ci/run.sh... it seems to never even run the AVX(2) tests with assert_instr turned on...?

@rustbot

rustbot commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@RalfJung

RalfJung commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Ah... it's 5 instructions somewhere. That's annoying.

disassembly for stdarch_test_shim__mm256_packs_epi16_vpacksswb: 
	 0: pushq %rbp
	 1: movq %rsp, %rbp
	 2: vpacksswb %ymm1, %ymm0, %ymm0
	 3: popq %rbp
	 4: retq

Maybe it should ignore some of these as they have nothing to do with the actual intrinsic?

@RalfJung RalfJung mentioned this pull request Aug 20, 2026
Open
@sayantn

sayantn commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

ci/run.sh runs the instruction assertions with only SSE2 enabled, because otherwise sometimes LLVM generates different assembly (e.g. if AVX is enabled, some SSE intrinsics will generate AVX instructions instead)

@RalfJung

RalfJung commented Aug 25, 2026

Copy link
Copy Markdown
Member Author

Yeah at some point I realized that this is about which instructions are enabled statically, but it'll still actually generate code for all intrinsics and even run assert_instr for all of them, even the ones my CPU could not execute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@folkertdev folkertdev

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@RalfJung @rustbot @sayantn @folkertdev